Skip to content

Conversation

@frankschlegel
Copy link
Member

Created an EDR brightness and wide gamut color test pattern image that can be used, for instance, to verify colors are displayed correctly, filters conserve those color properties correctly, etc.

…ions into test-pattern

# Conflicts:
#	README.md
#	Sources/CIColor+Extensions.swift
#	Sources/CIImage+Blending.swift
#	Sources/CIImage+Text.swift
#	Sources/CIImage+Transformation.swift
#	Sources/CIKernel+MetalSource.swift
#	Tests/ColorExtensionsTests.swift
#	Tests/ImageTransformationTests.swift
@frankschlegel frankschlegel requested a review from maikz August 9, 2022 10:53
@frankschlegel frankschlegel requested a review from phjs August 9, 2022 10:53
Copy link

@maikz maikz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed parts of it. The rest will follow soon

// Note: We reverse here so the first color specified in the array is displayed on the top.
for (row, color) in rowColors.reversed().enumerated() {
var swatch = CIImage.colorSwatch(for: color)
swatch = swatch.moved(to: CGPoint(x: CGFloat(column) * (swatch.extent.width + self.margin), y: CGFloat(row) * (swatch.extent.height + self.margin)))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let swatch = CIImage.colorSwatch()
    .moved(to:)

or even

pattern = CIImage.colorSwatch()
    .moved(to:)
   .composited(over: pattern)

?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same below

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here (and with the label) is that I need the extent of the swatch in the moved(to:), so I can't really chain it.

/// Colors for which swatches should be rendered, ordered by columns (outer) and rows (inner arrays).
private static var swatchColors: [[CIColor]] { [[.red, .green, .blue], [.cyan, .magenta, .yellow]] }
/// The different color spaces in which colors should be rendered in the color swatches.
private static var swatchColorSpaces: [(label: String, colorSpace: CGColorSpace)] = [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to have some kind of Swatch type with label, color space and color?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dict here is only needed to assign labels to the different color spaces. What would the Swatch type replace then?

// Put a label in the bottom left corner over the tile that displays the brightness value (also in nits).
let labelText = String(format: "%.2f (%d nits)", brightness, Int(brightness * 500))
let labelColor = CIColor(extendedWhite: brightness)!.contrastOverlayColor
let label = CIImage.text(labelText, fontName: self.labelFont, fontSize: size.height / 8.0, color: labelColor)!
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to be very safe we can add a fontScaleFactor since we divide by 8 multiple times.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And one time / 7.0. 😅
I didn't want to over-engineer this more than I already did. Otherwise, I'll end up writing a layout system for Core Image. 😬

@frankschlegel frankschlegel requested a review from maikz August 18, 2022 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants